Skip to content

feat(tIED): allow IED removal (closes #91).#92

Merged
JakobVogelsang merged 1 commit into
openscd:mainfrom
danyill:issue-91-remove-ied
Apr 1, 2024
Merged

feat(tIED): allow IED removal (closes #91).#92
JakobVogelsang merged 1 commit into
openscd:mainfrom
danyill:issue-91-remove-ied

Conversation

@danyill
Copy link
Copy Markdown
Collaborator

@danyill danyill commented Mar 29, 2024

Closes #91

As always, I would be very grateful for a review.

I'm not removing supervisions using the inbuild unsubscribe functionality because it's implemented "the wrong way" (removal of LNs or DOIs when I want removal of the GoCBRef/SvCBRef > Val's text content). I hope that's acceptable for now.

While I could mostly reuse the test structure for the updateIED code (thank you!), I have redone the test files as I needed the subscriptions and supervisions to be more realistic.

Once we have this right, I'd be keen for a new release as I have a menu plugin that I would like to provide in our distribution to allow IED removal.

@danyill
Copy link
Copy Markdown
Collaborator Author

danyill commented Mar 30, 2024

Currently, this does not remove empty SubNetwork elements if we are removing the last ConnectedAP.

I guess we could do something like the following but I worry about over-reach. And I guess we could make it an option. Perhaps we could just wait until it's a problem for someone 😉

function removeWithIedName(ied: Element, iedName: string): Remove[] {
  const selector = elementsToRemove
    .map((iedNameElement) => `${iedNameElement}[iedName="${iedName}"]`)
    .join(",");

  return Array.from(ied.ownerDocument.querySelectorAll(selector))
    .filter(isPublic)
    .map((element) => {
      if (
        element.tagName === "ConnectedAP" &&
        element.parentElement!.querySelectorAll("ConnectedAP").length === 1
      ) {
        return { node: <Element>element.parentElement };
      }
      return { node: element };
    });
}

@danyill danyill marked this pull request as ready for review March 30, 2024 00:23
@danyill danyill requested a review from JakobVogelsang March 30, 2024 00:23
Comment thread tIED/removeIED.ts Outdated
Comment thread tIED/removeIED.ts Outdated
Comment thread tIED/removeIED.ts Outdated
Comment thread tIED/removeIED.ts Outdated
Copy link
Copy Markdown
Contributor

@JakobVogelsang JakobVogelsang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was a pleasure to review. Thank you. Well structure and nice tests. There are just very minor issue that I have spotted.

@JakobVogelsang
Copy link
Copy Markdown
Contributor

I think an issue marking the fact that the removeIed does not clean the DataTypeTemplates section would also be nice. I don't think we should have that from the beginning, but a reminder is nice to have.

@danyill danyill force-pushed the issue-91-remove-ied branch from ed98dd9 to 17992a6 Compare March 30, 2024 05:35
@danyill
Copy link
Copy Markdown
Collaborator Author

danyill commented Apr 1, 2024

Thank you for this review, I have made some updates.

@danyill danyill requested a review from JakobVogelsang April 1, 2024 09:30
Copy link
Copy Markdown
Contributor

@JakobVogelsang JakobVogelsang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thx

@JakobVogelsang JakobVogelsang merged commit 16ddeb8 into openscd:main Apr 1, 2024
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tIED: Remove IED

2 participants